-
Notifications
You must be signed in to change notification settings - Fork 546
Instructions #365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instructions #365
Conversation
It was disturbing my sense of aesthetics
Tests pass locally, and I cannot see anything wrong with the one that fails in CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @zmeggyesi , thanks for the effort. However the instructions are already included in the InitializeResponse. You do not need a getter for this.
The public getter still might be useful for consistency and debugging.
I've left comments as you made some unnecessary changes.
Alos tests are missing
@@ -25,7 +25,7 @@ | |||
<dependency> | |||
<groupId>io.modelcontextprotocol.sdk</groupId> | |||
<artifactId>mcp</artifactId> | |||
<version>0.11.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change back to 0.11.0-SNAPSHOT
@@ -25,7 +25,7 @@ | |||
<dependency> | |||
<groupId>io.modelcontextprotocol.sdk</groupId> | |||
<artifactId>mcp</artifactId> | |||
<version>0.11.0-SNAPSHOT</version> | |||
<version>0.12.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change back to 0.11.0-SNAPSHOT
@@ -24,7 +24,7 @@ | |||
<dependency> | |||
<groupId>io.modelcontextprotocol.sdk</groupId> | |||
<artifactId>mcp</artifactId> | |||
<version>0.11.0-SNAPSHOT</version> | |||
<version>0.12.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change back to 0.11.0-SNAPSHOT
@@ -10,6 +10,7 @@ | |||
</parent> | |||
<artifactId>mcp</artifactId> | |||
<packaging>jar</packaging> | |||
<version>0.12.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change back to 0.11.0-SNAPSHOT
@@ -35,6 +35,7 @@ | |||
import org.slf4j.LoggerFactory; | |||
import reactor.core.publisher.Flux; | |||
import reactor.core.publisher.Mono; | |||
import reactor.util.annotation.Nullable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the reactor Nullable.
* Get the server instructions if available | ||
* @return The preset instructions for communication with the server | ||
*/ | ||
@Nullable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the reactor Nullable.
@@ -7,6 +7,7 @@ | |||
import io.modelcontextprotocol.spec.McpSchema; | |||
import io.modelcontextprotocol.spec.McpSchema.LoggingMessageNotification; | |||
import io.modelcontextprotocol.util.Assert; | |||
import reactor.util.annotation.Nullable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the reactor Nullable.
* Get the server instructions if available | ||
* @return The preset instructions for communication with the server | ||
*/ | ||
@Nullable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the reactor Nullable.
Closing as nothing to do. |
Motivation and Context
As a follow-up of #99 this PR provides access to the instructions as a readable field so they can be returned to the client during initialization
How Has This Been Tested?
Breaking Changes
No.
Types of changes
Checklist
Additional context
closes #364